execution, db: bind cache views to state versions and file views - #23095
Closed
yperbasis wants to merge 57 commits into
Closed
execution, db: bind cache views to state versions and file views#23095yperbasis wants to merge 57 commits into
yperbasis wants to merge 57 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors execution/cache so StateCache is published and consumed strictly as a single durable-state snapshot keyed by PlainStateVersion (state version), replacing the prior mix of applied-frontier tracking plus per-entry epoch/floor coherence. The goal is to close stale-fill windows around unwinds/commits by making cache usability contingent on an exact state-version match, and by revoking read views before any publication mutates cache contents.
Changes:
- Introduce a version-bound
cache.ReadView/publication token model (Publisher/Publication) and remove StateCache per-entry epoch/floor + applied-frontier coherence. - Rewire canonical vs speculative cache ownership (
SetCanonicalStateCachevsSetStateCacheReader) and update execution paths (FCU, SetHead, integration runner) accordingly. - Update read-ahead warmup and tests/benchmarks to use state-version-bound cache views and new step-based cache APIs.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| execution/vm/contract.go | Update jumpdest cache usage to new Put signature. |
| execution/execmodule/set_head.go | Switch SetHead path to canonical cache publication API. |
| execution/execmodule/forkchoice.go | Switch FCU canonical contexts to canonical cache publication API; remove warmup draining. |
| execution/execmodule/exec_module.go | Remove read-ahead draining helper; use reader-only cache attachment in ValidateChain. |
| execution/exec/blocks_read_ahead.go | Bind read-ahead warmup fills to PlainStateVersion and domain visibility; fill using kv.Step. |
| execution/exec/blocks_read_ahead_test.go | Adapt warmup tests to version-bound ReadView and publisher initialization. |
| execution/cache/view.go | Redefine ReadView as a durable-state-version handle with generation checks around reads/fills. |
| execution/cache/state_cache.go | Implement generation token, Publisher/Publication, step-based updates, and version-gated View. |
| execution/cache/generic_cache.go | Remove unwind coherence from GenericCache; store (value, step) for domain caches. |
| execution/cache/generic_cache_concurrency_test.go | Update concurrency tests for new Put/PutIfAbsent signatures and semantics. |
| execution/cache/code_cache.go | Remove unwind coherence; make code layers generation-cleared; switch stamping from txNum/epoch to step where needed. |
| execution/cache/code_cache_concurrency_test.go | Update concurrency tests for new code-cache semantics (clear-fencing only). |
| execution/cache/code_cache_codehash_test.go | Replace unwind-based tests with clear/publication-based expectations; update APIs. |
| execution/cache/cache.go | Update Cache interface to step-based API and simplify package-level docs. |
| execution/cache/cache_test.go | Update StateCache/DomainCache/CodeCache tests to publisher-based publication model and step API. |
| db/state/execctx/statecache_rpc_integration_test.go | Add/adjust integration tests covering unwinds and RPC views under version-bound cache publication. |
| db/state/execctx/statecache_readfill_test.go | Rewrite read-fill/unwind tests to new inactive-during-publication behavior and version-bound views. |
| db/state/execctx/statecache_readfill_bench_test.go | Remove frontier memo benchmark; align benchmark description with generation-check model. |
| db/state/execctx/flush_storage_cache_test.go | Update storage-cache commit callback test to read via current version-bound cache view. |
| db/state/execctx/export_test.go | Adjust exported test helpers to use version-bound cache view wiring. |
| db/state/execctx/domain_visible_end_memo_test.go | Remove now-obsolete visible-end memo concurrency tests. |
| db/state/execctx/domain_shared.go | Replace frontier memo + applier model with version-bound view selection and canonical Publisher publication pipeline. |
| db/state/execctx/codehash_routing_test.go | Update derived codehash routing tests to new generation safety (cache-sourced record may seed mapping). |
| cmd/integration/commands/stages.go | Wire integration stage runner to canonical cache publication API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
yperbasis
changed the title
yperbasis
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 45 out of 45 changed files in this pull request and generated no new comments.
Suppressed comments (2)
db/state/aggregator.go:1931
- When a commitment files publication triggers a BranchCache clear (BeginFilesPublication returns a non-nil backing-change), the AdaptivePinController should be reset as well; otherwise the controller will keep residency/miss state for pins that were just cleared and may make promotion/demotion decisions based on stale cache contents.
// SharedDomains.Commit acquires cache publication in the same order.
if domain := a.d[kv.CommitmentDomain]; domain != nil && domain.branchCache != nil {
if commitmentVisible := visible.d[kv.CommitmentDomain]; commitmentVisible != nil {
publication.branch = domain.branchCache.BeginFilesPublication(visibleFiles(commitmentVisible.files).EndTxNum())
}
}
db/state/aggregator.go:742
- closeDirtyFilesNoReopen resets the commitment BranchCache but does not reset the paired AdaptivePinController. Since AdaptivePinController keeps per-contract residency state, leaving it intact after a BranchCache reset can make the controller treat removed pins as still resident (see AdaptivePinController.Reset doc) and delay re-promotion/extension decisions.
This issue also appears on line 1926 of the same file.
a.visibilityLoweringForbidden.Store(false)
if cd := a.d[kv.CommitmentDomain]; cd != nil && cd.branchCache != nil {
cd.branchCache.Reset()
}
…versioned-generation
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 64 out of 64 changed files in this pull request and generated no new comments.
Suppressed comments (1)
execution/cache/generation_gate.go:209
Initializecan regress a cache that already publishes a newer state version. For example, a delayedSetCanonicalCachesfrom version 2 can clear and publish version 2 after another owner published version 3; fills from that old transaction can then survive into a later version-4 publication when version 3 changed the same keys. Reject older initialization just asPublishrejects older post-commit transitions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #22463.
Fixes #23028.
This supersedes #23005 while retaining its regression scenarios and folds #23047 into the same cache-publication model. #23139 describes a possible selective-invalidation optimization for canonical unwinds; it is not required for this correctness fix.
Problem
StateCacheandBranchCacheare process-global caches of latest state, while their callers read through transactions pinned to particular database and immutable-file views. Cache correctness therefore depends on the complete backing view, not only on the cached key.PlainStateVersionor passing the downloaded values through cache updates. Existing positive or negative entries must not remain live under the new files view (StateCache: fills can go stale when a snapshot download extends file visibility with never-applied state #23028).Design
Both caches follow one invariant:
A StateCache generation contains the state version and the exclusive values-file ends for accounts, storage, and code. A BranchCache generation contains the state version and the commitment values-file end.
HasCacheableLatestViewdetermines whether a transaction's latest-state result can be identified by that state version and values-file view. For a history-enabled domain, values files must cover history-II; this rejects dependency-clamped views whose backing can change without a database version change. A history-disabled domain has no history-II coverage requirement and remains eligible.TxNumsInFilessupplies the exclusive values-file ends. Equal ends remain compatible when physical files are merged or repacked.Each publication allocates an immutable pointer token. Pointer identity revokes existing views even when the numeric generation repeats. A separate reset lineage invalidates publisher handles captured before a full reset, including when no generation is currently published.
Boundary behavior
Every boundary that changes the published identity revokes existing views. Reads racing that revocation become misses, and fills already admitted under the old identity finish before cache mutation begins.
SharedDomainsis detachedResetExecCommit publication
SharedDomains.Commituses the same durable order for both caches:The database commit happens before cache revocation, so a failed commit leaves the caches unchanged. Transactions opened after a successful commit have the new state version and cannot bind to the old generation; older transactions still read their own durable snapshot until publication revokes their cache views.
Reads compare their token before and after the cache lookup. Fills recheck it under the admission lock shared with publication. An older post-commit publication never applies its partial update set over a newer published version.
Abortrestores the previous token only when no cache mutation was applied.A full reset advances the publisher lineage while publication and fills are blocked.
Initialize, commit publication, and files publication all reject handles captured before that boundary. Only a publisher acquired afterwards can establish the next generation.Unwinds and whole-state reset
A speculative or local unwind immediately detaches its
SharedDomainsfrom both shared caches, so the rewound overlay cannot read or fill them. If thatSharedDomainshas canonical publication authority, its later successful commit clears both caches and publishes the rewound state under a new state version.The full clear is intentional: unwind diffs cannot prove which derived or adaptively pinned entries still belong to the canonical state. #23139 evaluates selective retention with change-diff tombstones while keeping full clearing as the correctness fallback.
ResetExecreplaces execution state outsideSharedDomains.Commit. It resets both caches before starting the replacement transaction, then wipes the execution tables and advancesPlainStateVersionatomically. Pre-reset publisher handles are inert, so delayed work cannot republish the cleared generation.Files publication
Aggregator.recalcVisibleFilesis the common boundary for downloads, reopen, merge, and dependency recalculation. It reconciles the caches before publishing the new immutable-files bundle and holds cache publication until the matching files view is visible.The caches track how far their own committed updates cover each relevant domain. A files extension within that coverage can retain entries. An extension beyond it cannot be proven compatible with the process's cache-update stream, so the affected cache is cleared. Any changed file end revokes views tied to the previous ends even when entries are retained.
A transaction pinned to the previous files cannot bind to the new cache generation. Binding StateCache after files are already visible performs the same reconciliation. While StateCache is bound, values-file and history-II visibility cannot move backwards for accounts, storage, or code. BranchCache handles a lower commitment-file end by clearing before publishing the matching generation.
Scope and trade-offs
The generation gate replaces the StateCache read-view epoch, the per-entry unwind epochs, floors, and transaction numbers in both caches, and the separate
execution/cache/coherencepackage.TxNumremains only on transient committed updates to advance file-provenance watermarks. Cached state entries retain their sourceStepbecause bounded reads still need it.kvcache.Performance
The cache-hit path remains lock-free and allocation-free. It adds one atomic pointer comparison before and after the existing lookup. Fills reject stale views before cloning or hashing values, then take the admission read lock for the final validity check. Reset lineage is checked only on publisher paths and adds no cache-hit work.
Cache-view construction calls
HasCacheableLatestViewfor the four relevant domains and reads four file ends from the transaction's pinned in-memory metadata. A transaction different from theSharedDomainsbase view also readsPlainStateVersiononce per constructed getter. No database cursor is opened for cache eligibility.On darwin/arm64 with an Apple M2 Max,
BenchmarkCacheGetterConstructionhad a median of 0.71 us/op over five 5,000-iteration runs:go test ./db/state/execctx -run '^$' -bench '^BenchmarkCacheGetterConstruction$' -benchtime=5000x -count=5Two workload-level costs remain to be measured:
Review guide
execution/cache/generation_gate.go: generation identity, token checks, fill admission, publication, reset lineage, and files publication.execution/cache/view.goandexecution/commitment/branch_cache_view.go: generation-bound StateCache and BranchCache APIs.db/state/execctx/domain_shared.go: deriving views from pinned transactions and publishing after canonical database commits.db/state/aggregator.go,execution/cache/state_cache.go, andexecution/commitment/branch_cache.go: immutable-files publication and provenance coverage.execution/stagedsync/rawdbreset/reset_stages.goandcmd/integration/commands/stages.go: whole-state reset and staged-unwind boundaries.Tests
execution/cache/cache_test.go,execution/cache/generation_gate_test.go, andexecution/cache/files_publication_test.go: generation matching, stale reads and fills, publication abort, reset-lineage fencing, canonical clear, and files reconciliation.execution/commitment/branch_cache_test.go,execution/commitment/branch_cache_absorb_test.go, andexecution/commitment/adaptive_pin_test.go: the equivalent BranchCache behavior, file provenance, and adaptive plans.db/state/execctx/statecache_readfill_test.go,db/state/execctx/statecache_rpc_integration_test.go, anddb/state/execctx/branch_cache_flush_test.go: speculative isolation, canonical unwind, old transactions, commit failure, and RPC-visible behavior.db/state/aggregator_align_test.go,execution/stagedsync/rawdbreset/reset_stages_test.go, andcmd/integration/commands/stages_test.go: atomic files visibility, cache binding, whole-state reset, and staged unwind.Local verification:
go test ./execution/cache ./execution/commitment ./db/state/execctx ./execution/stagedsync/rawdbreset -count=1go test ./db/state ./cmd/integration/commands -count=1go test -race --timeout 20m ./execution/cache ./execution/commitment ./db/state/execctx ./execution/stagedsync/rawdbreset -count=1make lintrepeatedlymake erigon integration